Skip to content

Conversation

@AlexanderYastrebov
Copy link
Contributor

@AlexanderYastrebov AlexanderYastrebov commented Jul 28, 2025

Add EncryptReader function to encrypt io.Reader (pull-based encryption) and use it in cmd/age to get rid of io.Copy.

Updates #644

@AlexanderYastrebov AlexanderYastrebov force-pushed the encrypt-reader branch 2 times, most recently from bf2ee21 to 0962d50 Compare July 28, 2025 14:54
Add EncryptReader function to encrypt io.Reader (pull-based encryption)
and use it in command to get rid of io.Copy.

Updates FiloSottile#644

Signed-off-by: Alexander Yastrebov <[email protected]>
@gabyx
Copy link

gabyx commented Sep 7, 2025

This PR is nice and we need it, would it make sense to add this?

func EncryptStream(src io.Reader, recipients ... Recipient) (dst io.Reader, error) {
	pr, pw := io.Pipe()
	return pr, EncryptReader(pw, src, recipients...)
}

We wrap it currently as: https://gitlab.com/data-custodian/custodian/-/blob/88e7389ec5b837672f2b76fd82aaed68a4145e72/components/lib-common/pkg/crypto/reader.go

@AlexanderYastrebov
Copy link
Contributor Author

This PR is nice and we need it, would it make sense to add this?

I think this won't work as writes to pipe will block without corresponding reads.

@AlexanderYastrebov
Copy link
Contributor Author

This PR adds a handy function but what you are looking for is the implementation of #644

@FiloSottile
Copy link
Owner

I'm not sure I follow why this API is needed, or why it requires an internals refactor.

Full pull-based encryption like in #644 I understand, and without an Encrypt(io.Reader) io.Reader API it requires a goroutine, as you point out.

On the other hand, this Encrypt(dst io.Writer, in io.Reader) API only saves a synchronous io.Copy and could be implemented efficiently as

func EncryptReader(dst io.Writer, src io.Reader, recipients ...Recipient) error {
	w, err := age.Encrypt(out, recipients...)
	if err != nil {
		return err
	}
	if _, err := io.Copy(w, in); err != nil {
		return err
	}
	return w.Close()
}

without any other changes, right? Or am I missing something?

@AlexanderYastrebov
Copy link
Contributor Author

only saves a synchronous io.Copy

yes, it avoids unnecessary buffering and copying. I'll close this to avoid confusion.

@AlexanderYastrebov AlexanderYastrebov deleted the encrypt-reader branch December 25, 2025 01:07
@FiloSottile
Copy link
Owner

Oh I see, it avoids one of the copies inside io.Copy. We could do that also by implementing ReaderFrom / WriterTo, right? If you’re hitting a throughput bottleneck, I can add that to the I/O improvements I’m working on.

@AlexanderYastrebov
Copy link
Contributor Author

If you’re hitting a throughput bottleneck

TBH, I did not, I just saw a possible optimisation. Making stream.Writer implement ReaderFrom is the better way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants